Round-1 review on PR #538 surfaced 11 substantive findings; all addressed:

Critical (real bugs that broke compile or silently dropped logic):

 #1 [lint, refactor-plan] Rewrite target wrong — `wfctlhelpers.Plan` does
    not exist in the repo today. Pivoted to `platform.ComputePlan` (the
    real helper at platform/differ.go:72). Both targets now accepted by
    the lint analyzer for forward-compat with rev0 fixtures. Plan-doc
    §T8.3 named the wrong helper; flagged for retro.
 #2 [refactor-plan] rewritePlanBody only renamed `_` ctx params. A
    method declared `Plan(c context.Context, ...)` would be rewritten
    referencing undefined `ctx`. Now: any non-blank ctx-name preserved;
    only blank `_` renamed to `ctx`.
 #3 [refactor-plan] isCanonicalPlanBody too loose — extra side-effects
    inside the desired loop still classified as canonical. Tightened to
    require exactly the 3-statement template (lookup + !exists guard +
    configHash compare), no else branches, no trailing junk. Regression
    test: TestRefactorPlan_ExtraLoggingNotCanonical.
 #4 [refactor-plan, refactor-apply] SkipMarker only consulted on
    fn.Doc. PR description promised type-doc + GenDecl-doc honoring.
    Added receiverTypeDocs + carriesMarker; both modes now check all 3
    doc levels.
 #5 [refactor-apply] hasCanonicalCases only checked case labels. Bespoke
    bookkeeping inside a case body (logging, metrics, alternate driver
    calls) classified as canonical and would be silently dropped on
    -fix. Added caseBodyIsCanonical whitelist (driver call, ResourceRef
    construction, ProviderID guard). Regression test:
    TestRefactorApply_ExtraBookkeepingNotCanonical.
 #6 [refactor-apply] custom-error-wrapping suggestion named fictional
    APIs (ApplyResultErrorHook / WrapActionError). Replaced with honest
    hand-port advice: skip-marker + manual switch, OR move wrap into
    driver methods so wfctlhelpers records it verbatim.
 #7 [add-validate-plan] Stub always emitted unqualified `*IaCPlan` /
    `[]PlanDiagnostic`. Files importing the interfaces module under a
    qualifier (e.g. `*interfaces.IaCPlan`) failed to compile after
    -fix. Added interfacesQualifier detector + qualified stub emission.
    Regression: TestAddValidatePlan_Fix_QualifiedSignature.
 #8 [add-validate-plan, lint] hasValidatePlanMethod /
    AssertProviderImplementsValidatePlan checked method NAME only.
    Wrong-signature ValidatePlan (e.g. takes a string) was treated as
    compliant even though interfaces.ProviderValidator wouldn't be
    satisfied. Added validatePlanSignatureMatches: shape-checks the
    receiver param + return slice (qualified-or-unqualified). Both
    callers now use it. Regression:
    TestAddValidatePlan_DryRun_FlagsWrongSignature.
 #9 [refactor-plan, refactor-apply, add-validate-plan] Single-file
    pass — providers whose Plan + Apply lived in sibling files were
    silently omitted. Added planLikeReceiversInDir: directory-wide
    method-set scan. Per-file fallback retained for isolated single-
    file targets.

Important:

#10 [lint] Per-file parse/type-check errors accumulated in
    report.errors but exit code stayed 0 if there were no findings —
    green CI hid coverage gaps. Now exits 1 on either findings OR
    errors.
#11 [refactor-apply] -report-file mode flag never appeared in usage
    text. Documented in main.go's global usage block (the `-h` path
    intercepts before the per-mode FlagSet).

Plan-doc gap surfaced for retro: §T8.3 line 2373 reads "replaces with
`return wfctlhelpers.Plan(ctx, p, desired, current)`", but no such
function exists; reality is `platform.ComputePlan`. Recurring defect
class (plan-literal vs reality gap, W-4/W-5/W-7/W-9/W-8). Documented
in planHelperImportPath docstring + this commit body.

Round 2 surfaced 9 substantive findings; all addressed:

Critical (compile-break / contract-break):

 #1 [refactor-plan, lint] platform.ComputePlan returns IaCPlan BY VALUE,
    but provider Plan methods return *IaCPlan. Single-statement
    `return platform.ComputePlan(...)` rewrite produced uncompilable
    code. Switched to canonical 2-statement form:
        plan, err := platform.ComputePlan(ctx, p, desired, current)
        return &plan, err
    isAlreadyDelegatedPlanBody widened to recognise both the new shape
    and the legacy single-statement forms (idempotent across revs).
 #3 [refactor-plan] rewritePlanBody fell back to recvName="p" but
    didn't update the receiver decl when the source had an unnamed
    receiver (`func (*Provider) Plan(...)`). Rewritten call referenced
    undefined `p`. Added ensureReceiverName: injects identifier and
    mutates the AST. Regression: TestRefactorPlan_Fix_UnnamedReceiverGetsName.
    Also added: TestRefactorPlan_Fix_PreservesCustomCtxName for round-1
    finding #2 (custom ctx name preserved).
 #4 [refactor-apply] Same unnamed-receiver bug as #3. Same fix
    (ensureReceiverName + ensureCtxParamName + ensureNthParamName
    helpers shared with refactor-plan). Regression:
    TestRefactorApply_Fix_UnnamedReceiverGetsName.
 #5 [add-validate-plan] Stub always emitted `func (p *T) ValidatePlan(...)`
    even when the type used value receivers. Method-set mismatch made
    the type fail interfaces.ProviderValidator type assertion. Added
    providerReceiverConvention + receiverIsPointer; stub now matches
    the existing Plan/Apply convention. Regression:
    TestAddValidatePlan_Fix_ValueReceiverConvention.

Important (skip-marker not honored in lint, single-file pass):

 #6 [lint] AssertPlanDelegatesToHelper checked fn.Doc only, ignoring
    type-doc and GenDecl-doc skip markers. Added receiverTypeDocsForPass
    helper; analyzer now checks all 3 doc levels.
 #7 [lint] AssertApplyDelegatesToHelper — same fix as #6.
 #8 [lint] AssertDiffSetsNeedsReplaceForForceNew — same fix as #6.
 #9 [lint] lintFile passed only the target file to the analyzers, so
    cross-file method sets were invisible (same blind spot the
    refactor-* modes had in round 1). Now lintFile loads sibling
    non-test .go files from the same package directory and feeds the
    full slice to each analyzer; diagnostics for sibling files are
    dropped (the outer walker visits them in their own turn) so no
    duplicate findings.

All 4 modes now compile-clean rewrites + honor 3-level skip-marker +
package-aware method-set detection.

Round 3 surfaced 7 substantive findings; all addressed:

Critical (compile-break / silent data loss):

 #1 [add-validate-plan] Directory-wide detection only widened `provs`
    in round 2; methodsByRecv stayed file-local. A provider with
    ValidatePlan in a sibling file (or value-receiver Plan/Apply
    declared elsewhere) would receive a duplicate or wrong-receiver
    stub. Now planLikeProviderMethodsInDir returns both the recv set
    AND the merged method slice; methodsByRecv carries the
    package-wide view (deduped by method name). Stub injection still
    only fires when typeDecls[recv] is non-nil so we never append
    to a sibling file.
 #2 [refactor-plan] isCanonicalPlanBody accepted ANY 2-result return
    statement at the trailing slot. A planner with the canonical
    scaffold but a bespoke return (cloned plan, propagated error
    value) would classify as canonical and the bespoke logic would be
    silently dropped. Tightened to require EXACTLY `return plan, nil`.
 #3 [refactor-plan] rewritePlanBody hardcoded "desired"/"current" as
    args. A canonical Plan with renamed params (e.g. `Plan(ctx,
    specs, state)`) would rewrite to references to undefined
    identifiers. ensureNthParamName now extracts the actual signature
    names.
 #4 [refactor-plan] rewritePlanBody hardcoded "platform" as the call
    selector. A file using `pf "github.com/.../platform"` wouldn't
    compile because `platform` is undefined (ensureImport sees the
    aliased import as satisfying the path check). Added pkgAliasFor
    helper; rewrite now uses whatever local name the file imports
    under.
 #5 [refactor-apply] caseBodyIsCanonical accepted ANY AssignStmt as
    canonical. Bookkeeping AssignStmts (metrics counters, map
    updates, accumulators) passed and would be silently dropped.
    Tightened to a narrow whitelist: multi-target driver call,
    single-target driver call (LHS=err), composite-literal
    construction, selector-assignment to ResourceRef-style fields
    (ProviderID/Name/Type). Anything else rejected.
 #6 [refactor-apply] Same import-alias issue as #4 for `wfctlhelpers`.
    pkgAliasFor reused; rewriteApplyBody now uses whatever local name
    the file imports under.

Important:

 #7 [lint] AssertProviderImplementsValidatePlan checked ts.Doc only,
    missing markers placed on the wrapping GenDecl. Aligns now with
    the receiverDoc.carriesMarker pattern used by the other 3
    analyzers (round-2 #6/#7/#8). typeDocsByName captures both
    TypeSpec.Doc and GenDecl.Doc.

Round-2 regression tests retained (TestRefactorPlan_Fix_UnnamedReceiverGetsName,
TestRefactorPlan_Fix_PreservesCustomCtxName,
TestRefactorApply_Fix_UnnamedReceiverGetsName,
TestAddValidatePlan_Fix_ValueReceiverConvention).
Round-3 fix verified end-to-end against an aliased-import fixture
(pf "github.com/.../platform" + wfh "github.com/.../wfctlhelpers"):
the rewritten output compiles cleanly under gofmt.

Round 5 surfaced 9 findings; all addressed. Recurring theme: the
detectors and reporters needed deeper structural verification (branch
contents, outer-shape, receiver-kind, package isolation, exit-code
semantics) — not just shape matching at one level.

Critical (silent data loss / repair regression):

 #1 [refactor-plan] rangeBodyMatchesCanonicalDesired only checked the
    guard expressions and statement count; never inspected what the
    `!exists` and `configHash != configHash` branch BODIES did. A
    planner with extra logic (telemetry, alternate action construction,
    different create/update payload) inside those branches was
    silently rewritten away. Added isCanonicalCreateBranchBody +
    isCanonicalUpdateBranchBody + isPlanActionsAppendAssign to verify
    the create branch is exactly `append+continue` and the update
    branch is exactly `append`.
 #2 [refactor-apply] classifyApplyBody verified only the switch
    shape; setup/teardown/result aggregation OUTSIDE the switch was
    silently dropped on -fix. Added isCanonicalApplyOuterShape: the
    Apply body must be exactly the 3-statement scaffold (result-init
    + range-loop + return result, nil).
 #3 [add-validate-plan] hasValidatePlanMethod ignored receiver kind.
    A value-receiver provider with a pointer-receiver ValidatePlan
    still failed the ProviderValidator type assertion (method-set on
    `T` does not include `*T` methods), but rev2 treated it as
    already-implemented. Now also requires receiver-kind match.
 #4 [lint] AssertProviderImplementsValidatePlan had the same
    receiver-kind blind spot. Now delegates to hasValidatePlanMethod
    (centralised + DRY).
 #5 [refactor-plan] isAlreadyDelegatedPlanBody accepted single-statement
    `return platform.ComputePlan(...)` (broken rev1 form) as
    already-delegated, so rerunning the fixed codemod never repaired
    output from the earlier broken rewrite. Now ONLY accepts the
    canonical 2-statement form; broken single-statement forms classify
    as non-canonical so a fresh -fix produces compilable output.
 #6 [refactor-plan] planLikeProviderMethodsInDir merged methods from
    every non-test .go file regardless of `package P` clause. Mixed-
    package or build-tagged directories could fold methods from
    unrelated packages into a synthetic provider. Added two-pass
    package-clause check: aggregate only files matching the dominant
    package.

Important (CI fidelity / detector recall):

 #7 [Makefile, lint] `|| true` in migrate-providers swallowed real
    execution failures alongside expected advisory findings, because
    lint returned 1 for both findings AND parse errors. Split the
    exit codes: 0 clean / 1 findings / 2 errors. Makefile now gates
    on `[ $? -ne 0 ] && [ $? -ne 1 ]` so parse errors fail the
    target.
 #8 [refactor-plan] Canonical matcher hardcoded the lookup flag
    name as `exists`. The semantically-identical `cur, ok :=`
    idiomatic Go form was reported non-canonical. Widened to accept
    both `exists` and `ok`.
 #9 [refactor-apply] isDriverMethodCall allowlist {d, drv, driver}
    missed common alternates. Widened to {d, dr, drv, rdrv, driver,
    resourceDriver}. Still rejects bookkeeping receivers like
    `metrics`, `audit`, `helper` (preserves round-4 #3 fix).

End-to-end verification: lint against DO plugin produces exit 1 (3
advisory findings, no errors); broken-Go-source produces exit 2;
clean source produces exit 0. Smoke-tested via /tmp/iac-codemod.

…ening findings

Round 7 surfaced 10 findings; 4 were stale (already fixed in R6). 6
real findings addressed:

Critical (compile-break / silent data loss):

 #1 [refactor-plan] isPlanActionsAppendAssign verified the LHS but
    not append's first argument. A bespoke `plan.Actions =
    append(otherSlice, ...)` was misclassified as canonical and the
    alternate-slice logic silently dropped during rewrite. Now both
    LHS and append's first arg must reference plan.Actions.
 #3+#9 [refactor-apply] isCanonicalApplyOuterShape only checked the
    outer 3-statement scaffold; per-action logic INSIDE the for-loop
    body (logging, metrics, custom error handling, accumulators) was
    silently dropped on -fix. Added isCanonicalApplyLoopBody +
    isCanonicalApplyLoopAssign + isCanonicalApplyLoopIf +
    isCanonicalApplyLoopIfBodyStmt: every loop-body statement must
    match a tight whitelist (driver lookup, var-out decl, action
    switch, err-/out-guard ifs).
 #7+#8 [add-validate-plan] provs[recv].Pos() panicked when the
    TypeSpec was nil (cross-file scenario from round-3 #1: type
    declaration in sibling file). Now defaults Pos to NoPos for nil
    specs; sort still works (stable on name when Pos ties).

Important (cross-file consistency):

 #4 [add-validate-plan] qualifier fallback to "interfaces" fired
    based on whether ANY sibling imported interfaces — unreliable
    if THIS provider uses local types but an unrelated sibling
    imports interfaces. Replaced with qualifierFromProviderMethods:
    inspects the provider's OWN Plan/Apply parameter types
    (directory-wide via round-3 #1) for the qualifier they use.
 #5 [add-validate-plan] skip-marker check only consulted typeDecls
    (current file). When Plan/Apply are here but the type with
    `// wfctl:skip-iac-codemod` lives in a SIBLING file, the marker
    was ignored. Added siblingTypeDocs lookup via
    receiverTypeDocsInDir (the round-6 helper).
#10 [add-validate-plan] sibling-method merge deduped by method NAME
    only. If local file has wrong-signature ValidatePlan and sibling
    has correct one, sibling dropped, hasValidatePlanMethod saw only
    bad declaration, injected duplicate stub. Replaced with
    isLocalDuplicate: dedupes by name + parameter arity + result
    arity, so distinct signatures both survive.

Stale findings (already fixed in R6, no action needed):
 #2 refactor-apply receiverTypeDocsInDir already in place
 #6 lint receiver-doc lookup already merged via receiverTypeDocsForPass

Smoke-tested against DO plugin: refactor-plan reports DOProvider.Plan
canonical, refactor-apply reports DOProvider.Apply upsert-recovery
with the upsertSupporter suggestion. Output matches T8.7 baseline.

…fier-name findings

Round 8 surfaced 9 findings; all addressed:

Critical (silent data loss / behavior change):

 #1 [add-validate-plan] isLocalDuplicate compared by name+arity only.
    Wrong-signature ValidatePlan(name string) []PlanDiagnostic and
    correct ValidatePlan(plan *IaCPlan) []PlanDiagnostic have same
    arity but different types — sibling-correct dropped, duplicate
    stub injected. Replaced with signature-fingerprint dedupe
    (signatureFingerprint + typeFingerprint walk all type shapes).
 #4 [refactor-apply] `default:` case clauses accepted without body
    inspection. Logging/metrics in default body silently dropped.
    Added isCanonicalDefaultBody: only `err = fmt.Errorf("unknown
    action %q", ...)` accepted.
 #5 [refactor-apply] isCanonicalApplyLoopAssign accepted any
    `<x>.ResourceDriver(...)`. `helper.ResourceDriver(...)` /
    `plan.ResourceDriver(...)` falsely classified. Now requires the
    receiver to match the provider's own receiver identifier
    (threaded through from classifyApplyBody).
 #8 [refactor-apply] Bare `if err != nil { continue }` accepted as
    canonical, but wfctlhelpers ALWAYS records ActionError before
    continuing — the rewrite would silently change behavior. Now
    requires the if-body to ALSO append to result.Errors before any
    continue/break.

Important (skip-marker scope + identifier flexibility):

 #2 [add-validate-plan] Skip-marker check fired on EVERY method's
    fn.Doc — a marker on Destroy/Status/etc. accidentally suppressed
    the whole provider's analysis. Restricted to Plan/Apply (the
    provider-defining methods).
 #3 [lint] AssertProviderImplementsValidatePlan — same fix as #2.
 #6 [refactor-plan] Canonical detector hardcoded `current`/`desired`
    body identifiers. Providers using `state`/`specs` reported
    non-canonical despite rewriter preserving names. Added
    nthParamName extraction; isCanonicalPlanBody now takes the actual
    parameter names.
 #7 [refactor-apply] Driver-receiver allowlist comment claimed `rd`
    accepted, but the switch was missing it. Added.
 #9 [refactor-apply] Canonical detector hardcoded `result` /`plan`
    identifier names. Providers using `res` /`pl` rejected. Now
    recovers actual identifier from signature (planName) and from
    statement-1 LHS (resultName); both must be consistent within the
    body but can be any identifier.

Smoke-tested against DO plugin: refactor-plan / refactor-apply still
report DOProvider.Plan canonical / DOProvider.Apply upsert-recovery
with stable upsertSupporter suggestion. Output matches T8.7 baseline.

Removed redundant TestRefactorApply_Fix_UnnamedReceiverGetsName: the
unnamed-receiver path can't have a canonical-shape Apply body
(`<recv>.ResourceDriver(...)` requires recv in scope). Receiver-name
injection is shared between refactor-plan and refactor-apply via
ensureReceiverName; coverage stays in
TestRefactorPlan_Fix_UnnamedReceiverGetsName.

Round 10 surfaced 8 findings; all addressed:

Critical (cross-file duplicate stub / silent override):

 #1 [add-validate-plan] Cross-file duplicate stub injection: when type
    is in file_a and Plan/Apply are in file_b, both files classified
    as missing-ValidatePlan and -fix injected duplicate stubs. Now
    only inject in the file containing the receiver TypeSpec
    (`if ts == nil { skip }`); the type-file's own pass handles it.
 #2 [add-validate-plan] Embedded-field promoted ValidatePlan not
    detected; -fix would shadow it with a no-op stub, silently
    dropping real plan diagnostics. Added typeHasEmbeddedFields:
    if the receiver type has any embedded fields, suppress the
    missing classification (we can't statically resolve method
    promotion without full type info, so err on the side of NOT
    injecting).
 #3 [lint] AssertProviderImplementsValidatePlan — same fix as #2.
 #4 [refactor-apply] ProviderID/Name/Type assignment-target whitelist
    didn't check struct identity. `audit.Type = ...` or
    `result.ProviderID = ...` (wrong struct) classified as canonical
    and dropped on rewrite. Now requires the LHS receiver to be
    `ref` (the canonical ResourceRef construction site name).

Important (perf / determinism / lint precision):

 #5 [lint] O(n²) lintFile re-parsed every sibling per-call. Added
    lintDirCache: lintPath now groups files by directory and builds
    one parse cache per dir, reused across the directory's files.
    Per-call fallback retained for single-file invocation.
 #6 [refactor-plan] planLikeProviderMethodsInDir's dominant-package
    selection used range-over-map (random iteration), so on a
    package-count tie the dominant could differ across runs and
    rewrite against the wrong method set. Sort the package names
    so tie-break is lexicographic-first (deterministic).
 #7 [lint] AssertPlanDelegatesToHelper accepted ANY platform.ComputePlan
    call ANYWHERE in the body. Now requires the canonical SHAPE:
    either the 2-statement rev2 form (matches isAlreadyDelegatedPlanBody)
    OR a single-statement legacy `return <X>.Plan(...)` /
    `return <X>.ComputePlan(...)`. Bespoke wrappers that call the
    helper as an intermediate step now correctly flag.
 #8 [lint] AssertApplyDelegatesToHelper — same fix: now uses
    isAlreadyDelegatedApplyBody (the rewriter's idempotency check)
    so anything but the canonical single-statement form flags.

Smoke-tested against DO plugin: refactor-plan / refactor-apply still
report DOProvider.Plan canonical / DOProvider.Apply upsert-recovery
with stable upsertSupporter suggestion. Output matches T8.7 baseline.

…tening findings

Round 12 surfaced 8 findings; all addressed:

Critical (CLI bug + silent rewrite of wrong file):

 #1 [main.go] Top-level dispatcher used a single FlagSet with only
    -dry-run and -fix registered, so any mode-specific flag (e.g.
    refactor-apply's -report-file) failed with "flag provided but
    not defined" BEFORE the mode could parse it. -report-file was
    documented but UNUSABLE from the CLI entrypoint. Replaced
    stdlib FlagSet with a manual-scan loop in run(): -dry-run/-fix
    are extracted; everything else (including unknown flags) flows
    through to the mode's own FlagSet. Bonus: flag-position
    flexibility (`/path -fix` now works), updated test +
    usage text accordingly.
 #2 [refactor-plan] Walked every .go file but built provs/typeDocs
    from only the dominant package. Mixed-package or build-tagged
    directories: a non-dominant file with overlapping receiver
    names was processed against another package's method set,
    rewriting the wrong file. Added dominantPackageForDir; each
    file processor now skips files in non-dominant packages.
 #3 [refactor-apply] Same fix as #2.
 #4 [add-validate-plan] Same fix as #2.

Important (canonical-detection precision):

 #5 [refactor-plan] isPlanActionsAppendAssign didn't validate the
    appended action's payload — `plan.Actions = append(plan.Actions,
    PlanAction{Action: "queue"})` was misclassified as canonical and
    silently rewritten. Added `expectedAction` parameter; create
    branch requires `Action: "create"` and update branch requires
    `Action: "update"`.
 #6 [refactor-apply] hasCanonicalCases verified case labels but not
    that the body's driver call MATCHED the label. A `case "create"`
    body that called `.Update()` or `.Delete()` was misclassified
    and silently rewritten away. Added caseBodyMatchesLabel: scans
    each case body for driver method calls and verifies the label-
    to-method mapping (create→Create, update→Update, delete→Delete,
    replace→Update).
 #7 [refactor-apply] Driver-lookup check accepted any
    `<recv>.ResourceDriver(<arg>)` regardless of <arg>. wfctlhelpers
    always dispatches with `action.Resource.Type`, so providers
    using a different lookup key (e.g. action.Tag, computed value)
    would see different driver behavior on rewrite. Now requires
    the lookup key to be exactly `action.Resource.Type`.
 #8 [lint] looksLikeProvider checked method NAMES + rough arity,
    so any unrelated type with `Plan(...)` and `Apply(...)` was
    treated as a provider (e.g., a deploy strategy). Tightened to
    verify signature shapes via type-name suffix matching:
    Plan must be `Plan(ctx, []ResourceSpec, []ResourceState)
    (*IaCPlan, error)` and Apply must be `Apply(ctx, *IaCPlan)
    (*ApplyResult, error)`. Qualified or unqualified accepted via
    typeNameTailMatches.

Smoke-tested:
- `iac-codemod refactor-apply -report-file <path> <dir>` now works
  (previously: "flag provided but not defined")
- DO plugin still reports DOProvider.Plan canonical / Apply
  upsert-recovery with stable upsertSupporter suggestion (T8.7
  baseline preserved)

…get (W-8 of 12) (#538)

* feat(codemod): scaffold cmd/iac-codemod with 4-mode subcommand dispatcher

T8.1: Adds cmd/iac-codemod skeleton with dispatcher for the four codemod
modes — refactor-plan, refactor-apply, add-validate-plan, lint — and the
shared -dry-run / -fix flag pair. Modes are registered via a map of
modeFunc entries so subsequent tasks (T8.2-T8.5) can wire in real
implementations file-by-file. Each mode currently delegates to a stub
that prints a "not yet implemented" message and exits zero.

Defaults: -dry-run is true; -fix opts into mutation and forces -dry-run
to false. Unknown modes return exit 2 with usage. The // wfctl:skip-iac-codemod
marker convention is documented in the package doc and usage text.

Tests cover dispatch, default flag values, -fix semantics, unknown-mode
handling, help routing, and positional-arg forwarding via a swappable
modes map (no subprocess required).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(codemod): pin SkipMarker const + document flag ordering (T8.1 review)

Addresses spec-reviewer findings on b76ab2f:

1. (BLOCKER) Extract `const SkipMarker = "// wfctl:skip-iac-codemod"` so
   T8.3-T8.5 parsers reference the canonical literal in one place. Plan
   rev2 (line 2400) unifies the four modes on this single marker
   specifically to prevent mismatched-marker silent-no-op surfaces; the
   const + TestSkipMarker_LiteralPinned + TestUsage_MentionsSkipMarker
   guards close the drift hole the reviewer flagged. usage() now formats
   the marker via the const rather than a duplicated string literal.

2. (MINOR) usage() documents the stdlib flag-parser ordering constraint
   (flags must precede paths). TestRun_FlagAfterPath_SilentlyTreatedAsPositional
   pins the failure mode so it is intentional, not a parser bug, and so
   future maintainers see the constraint exercised in tests.

3. (NIT) stubMode's unused args parameter renamed to _; cosmetic only.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(codemod): close -dry-run=false mutation-gate bypass (T8.1 review #4)

Spec-reviewer round-2 finding (commit 26ac916): the dispatcher only
forced DryRun=false on -fix, but did NOT prevent a user-supplied
-dry-run=false from leaving the gate open. With the natural mode
predicate `if !opts.DryRun { mutate() }`, this would silently bypass
the explicit -fix gate that plan §W-8 line 2347 names as the sole
mutation entry point ("-dry-run flag default true; -fix opts into
mutation").

Fix: normalize the gate at the dispatcher boundary — when Fix is set,
DryRun=false; when Fix is unset, DryRun=true regardless of what the
user passed via -dry-run=. Fix is now the single source of truth for
"may I mutate?", so any natural mode predicate is safe by construction.
Options.DryRun's doc comment now states this contract explicitly so
T8.2-T8.5 implementers cannot reach for the wrong predicate.

Tests pin all three cases:
  - -dry-run=false alone        → DryRun stays true (the bypass)
  - -fix -dry-run=false         → mutation authorized (Fix wins)
  - -dry-run=true -fix          → mutation authorized (Fix wins)

Also adds TestPackageDoc_MentionsSkipMarker (process note #6) — cheap
file-content guard so a future SkipMarker rename trips a test rather
than silently desyncing the package doc comment.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* docs(codemod): warn future maintainers off t.Parallel in main_test.go (T8.1 review #5)

Code-reviewer round-3 authorized now-fix: tests in this file mutate the
package-global `modes` map under defer-restore. -race is currently clean
because no test calls t.Parallel(), but the swap-and-restore pattern is
a latent data race the next agent (T8.2-T8.5) could trigger by adding
parallelism. Top-of-file guard comment names the constraint and points
at the dependency-injection refactor as the unlock path if parallelism
is ever required.

Comment-only change; tests still pass with -race.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat(codemod): lint mode with 4 static-check assertions

T8.2: Wires the lint subcommand using golang.org/x/tools/go/analysis
with the four assertions named in plan §T8.2:

  AssertPlanDelegatesToHelper                — provider Plan() delegates to wfctlhelpers.Plan
  AssertApplyDelegatesToHelper               — provider Apply() delegates to wfctlhelpers.ApplyPlan
  AssertDiffSetsNeedsReplaceForForceNew      — driver Diff() sets NeedsReplace on ForceNew
  AssertProviderImplementsValidatePlan       — provider satisfies ProviderValidator

Carry-forwards from T8.1 review baked in:

  1. Dispatcher fs.Usage override (main.go:run) so `iac-codemod <mode> -h`
     produces the global usage rather than the per-FlagSet banner.
     Pinned by TestRun_HelpAfterMode_PrintsGlobalUsage across all 4 modes.

  2. Mutation-gate negative test pinning lint-is-read-only-by-definition:
     TestRunLint_DoesNotMutateFilesEvenWithFixFlag invokes lint with
     hostile {Fix:true, DryRun:false} flags and asserts mtime + content
     unchanged. Plus TestRunLint_FixFlag_WarnsItHasNoEffect surfaces a
     warning so users know -fix did nothing.

  3. Skip-marker honored at func-doc and type-doc levels via
     hasSkipMarkerOn(fn.Doc) / ts.Doc; skipped sites flow through the
     pass.Report channel with a [skipped] prefix and are split into a
     separate report section by lintReport.unpackSkippedFromFindings.
     Plan rev2 (line 2400) requires each mode to surface a list of
     skipped sites in its report — pinned by TestRunLint_SkipMarker_SurfacedInReport.

Precision: all helper-call analyzers gate on providerLikeReceivers
(method set must contain BOTH Plan + Apply matching IaCProvider shape)
to avoid false-positive flags on deploy targets and other Apply-shaped
types. Manual verification against the workflow repo went from 9
findings (incl. 2 false positives in pkg/k8s) down to 7 (all genuine
provider implementations awaiting v2 migration).

Implementation notes:

  - File-by-file analysis via parser.ParseFile + tolerant types.Check
    (stub importer ignores unresolved imports). This works on plugin
    sources that haven't vendored their dependencies. Cross-file
    references won't resolve, but IaC providers and drivers are
    typically co-located by Go convention.
  - Skip-marker is encoded as a synthetic diagnostic with a `[skipped]`
    prefix; the driver post-processes it out of the findings list. This
    keeps the analyzer API surface to one channel.
  - go.mod: promotes golang.org/x/tools from indirect to direct. No
    new modules, no go.sum changes.

Verification: 33/33 tests pass with -race; binary smoke-tested against
workflow repo root (7 findings, exit 1).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(codemod): T8.2 review — stdout for -h, marker context, precision filter

Spec-reviewer round-2 on commit 2908fa1; addresses 5 substantive
findings + 1 nit (findings 5 & 6 are PR-body notes, no code change):

1. (BLOCKER) `iac-codemod <mode> -h` now prints the global usage to
   STDOUT, matching `iac-codemod -h` and the kubectl/git/gh convention
   for help-on-success. Previously it landed on STDERR via the
   FlagSet's SetOutput handler. Pinned by
   TestRun_HelpAfterMode_PrintsGlobalUsageToStdout — asserts stream
   specifically rather than the union of stdout+stderr (the prior test
   would have passed even with stderr output). Parse-error noise still
   flows through stderr; only the help-text body moved to stdout.

2. (MEDIUM) hasSkipMarkerOn now accepts a trailing space + arbitrary
   justification text after SkipMarker:
     // wfctl:skip-iac-codemod legacy upsert recovery, see ADR-042
   Annotating WHY a site is skipped is a Go idiom; silently ignoring
   the marker because of trailing context would replicate the exact
   silent-no-op surface plan rev2 line 2400 unifies the marker to
   prevent. Two new tests pin both sides of the contract:
     - TestSkipMarker_AcceptsTrailingJustification
     - TestSkipMarker_RejectsCloseButWrongMarker (negative — the
       legacy `// wfctl:skip-codemod` prefix from design rev1 must
       still flag the diagnostic)

3. (MEDIUM) AssertDiffSetsNeedsReplaceForForceNew now gates on a new
   driverLikeReceivers helper (method set must contain Diff AND at
   least one canonical companion: Read/Create/Update/Delete). Brings
   the analyzer in line with the precision treatment Plan/Apply
   already had via providerLikeReceivers. New
   TestAssertDiffSetsNeedsReplaceForForceNew_NonDriverNotFlagged pins
   the negative case (a SettingsDiff struct with just Diff() is
   correctly invisible to the analyzer).

4. (LOW-MEDIUM) bodyAssignsFieldTrue → bodyAssignsField: the matcher
   now accepts ANY RHS, not just literal `= true`. The terser canonical
   pattern `r.NeedsReplace = c.ForceNew` is equally valid expression
   of the W-3 force-new contract; flagging it was a false positive
   previously hit by cmd/wfctl/deploy_providers.go remoteResourceDriver
   (which propagates NeedsReplace from a gRPC response via
   `result.NeedsReplace, _ = res["needs_replace"].(bool)`). Pinned by
   TestAssertDiffSetsNeedsReplaceForForceNew_AcceptsDirectAssign.

7. (NIT) Removed dead/misleading comment in lintFile that referenced a
   never-implemented passSkippedSink scratch field.

Findings 5 & 6 (no code change — PR-body notes for team-lead):

  5. Plan §T8.2 line 2363 says `golang.org/x/tools/go/analysis/passes`
     framework, but `/passes` is the directory of canonical reusable
     analyzers. The actual framework is `golang.org/x/tools/go/analysis`
     (which is what we import). Likely a plan typo; flag for
     post-merge retrospective.

  6. go.mod promotes golang.org/x/tools from indirect to direct.
     Already-transitive dep, no go.sum changes, no new modules. Should
     be fine but flagged for team-lead per W-7 trigger-list rigor.

Smoke-test re-verification on workflow repo: 6 genuine findings (down
from 7), zero false positives. -h now correctly streams to stdout for
both top-level and per-mode invocations.

37/37 tests pass with -race; build clean; vet clean.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(codemod): T8.2 review round-2 — tab-delimited marker, literal-false guard, adjacent-suffix rejection

* feat(codemod): refactor-plan mode (canonical pattern detection + rewrite); honors // wfctl:skip-iac-codemod marker

* feat(codemod): refactor-apply with informative non-canonical idiom reports; honors // wfctl:skip-iac-codemod marker

* feat(codemod): add-validate-plan mode (no-op stub injection); honors // wfctl:skip-iac-codemod marker

* chore(make): add migrate-providers target for workspace-wide codemod

* fix(codemod): T8.7 verification — exclude _worktrees and other underscore-prefixed dirs from walk

* fix(codemod): Copilot review round 1 — 9 critical + 2 important findings

Round-1 review on PR #538 surfaced 11 substantive findings; all addressed:

Critical (real bugs that broke compile or silently dropped logic):

 #1 [lint, refactor-plan] Rewrite target wrong — `wfctlhelpers.Plan` does
    not exist in the repo today. Pivoted to `platform.ComputePlan` (the
    real helper at platform/differ.go:72). Both targets now accepted by
    the lint analyzer for forward-compat with rev0 fixtures. Plan-doc
    §T8.3 named the wrong helper; flagged for retro.
 #2 [refactor-plan] rewritePlanBody only renamed `_` ctx params. A
    method declared `Plan(c context.Context, ...)` would be rewritten
    referencing undefined `ctx`. Now: any non-blank ctx-name preserved;
    only blank `_` renamed to `ctx`.
 #3 [refactor-plan] isCanonicalPlanBody too loose — extra side-effects
    inside the desired loop still classified as canonical. Tightened to
    require exactly the 3-statement template (lookup + !exists guard +
    configHash compare), no else branches, no trailing junk. Regression
    test: TestRefactorPlan_ExtraLoggingNotCanonical.
 #4 [refactor-plan, refactor-apply] SkipMarker only consulted on
    fn.Doc. PR description promised type-doc + GenDecl-doc honoring.
    Added receiverTypeDocs + carriesMarker; both modes now check all 3
    doc levels.
 #5 [refactor-apply] hasCanonicalCases only checked case labels. Bespoke
    bookkeeping inside a case body (logging, metrics, alternate driver
    calls) classified as canonical and would be silently dropped on
    -fix. Added caseBodyIsCanonical whitelist (driver call, ResourceRef
    construction, ProviderID guard). Regression test:
    TestRefactorApply_ExtraBookkeepingNotCanonical.
 #6 [refactor-apply] custom-error-wrapping suggestion named fictional
    APIs (ApplyResultErrorHook / WrapActionError). Replaced with honest
    hand-port advice: skip-marker + manual switch, OR move wrap into
    driver methods so wfctlhelpers records it verbatim.
 #7 [add-validate-plan] Stub always emitted unqualified `*IaCPlan` /
    `[]PlanDiagnostic`. Files importing the interfaces module under a
    qualifier (e.g. `*interfaces.IaCPlan`) failed to compile after
    -fix. Added interfacesQualifier detector + qualified stub emission.
    Regression: TestAddValidatePlan_Fix_QualifiedSignature.
 #8 [add-validate-plan, lint] hasValidatePlanMethod /
    AssertProviderImplementsValidatePlan checked method NAME only.
    Wrong-signature ValidatePlan (e.g. takes a string) was treated as
    compliant even though interfaces.ProviderValidator wouldn't be
    satisfied. Added validatePlanSignatureMatches: shape-checks the
    receiver param + return slice (qualified-or-unqualified). Both
    callers now use it. Regression:
    TestAddValidatePlan_DryRun_FlagsWrongSignature.
 #9 [refactor-plan, refactor-apply, add-validate-plan] Single-file
    pass — providers whose Plan + Apply lived in sibling files were
    silently omitted. Added planLikeReceiversInDir: directory-wide
    method-set scan. Per-file fallback retained for isolated single-
    file targets.

Important:

#10 [lint] Per-file parse/type-check errors accumulated in
    report.errors but exit code stayed 0 if there were no findings —
    green CI hid coverage gaps. Now exits 1 on either findings OR
    errors.
#11 [refactor-apply] -report-file mode flag never appeared in usage
    text. Documented in main.go's global usage block (the `-h` path
    intercepts before the per-mode FlagSet).

Plan-doc gap surfaced for retro: §T8.3 line 2373 reads "replaces with
`return wfctlhelpers.Plan(ctx, p, desired, current)`", but no such
function exists; reality is `platform.ComputePlan`. Recurring defect
class (plan-literal vs reality gap, W-4/W-5/W-7/W-9/W-8). Documented
in planHelperImportPath docstring + this commit body.

* fix(codemod): Copilot review round 2 — 5 critical + 4 important findings

Round 2 surfaced 9 substantive findings; all addressed:

Critical (compile-break / contract-break):

 #1 [refactor-plan, lint] platform.ComputePlan returns IaCPlan BY VALUE,
    but provider Plan methods return *IaCPlan. Single-statement
    `return platform.ComputePlan(...)` rewrite produced uncompilable
    code. Switched to canonical 2-statement form:
        plan, err := platform.ComputePlan(ctx, p, desired, current)
        return &plan, err
    isAlreadyDelegatedPlanBody widened to recognise both the new shape
    and the legacy single-statement forms (idempotent across revs).
 #3 [refactor-plan] rewritePlanBody fell back to recvName="p" but
    didn't update the receiver decl when the source had an unnamed
    receiver (`func (*Provider) Plan(...)`). Rewritten call referenced
    undefined `p`. Added ensureReceiverName: injects identifier and
    mutates the AST. Regression: TestRefactorPlan_Fix_UnnamedReceiverGetsName.
    Also added: TestRefactorPlan_Fix_PreservesCustomCtxName for round-1
    finding #2 (custom ctx name preserved).
 #4 [refactor-apply] Same unnamed-receiver bug as #3. Same fix
    (ensureReceiverName + ensureCtxParamName + ensureNthParamName
    helpers shared with refactor-plan). Regression:
    TestRefactorApply_Fix_UnnamedReceiverGetsName.
 #5 [add-validate-plan] Stub always emitted `func (p *T) ValidatePlan(...)`
    even when the type used value receivers. Method-set mismatch made
    the type fail interfaces.ProviderValidator type assertion. Added
    providerReceiverConvention + receiverIsPointer; stub now matches
    the existing Plan/Apply convention. Regression:
    TestAddValidatePlan_Fix_ValueReceiverConvention.

Important (skip-marker not honored in lint, single-file pass):

 #6 [lint] AssertPlanDelegatesToHelper checked fn.Doc only, ignoring
    type-doc and GenDecl-doc skip markers. Added receiverTypeDocsForPass
    helper; analyzer now checks all 3 doc levels.
 #7 [lint] AssertApplyDelegatesToHelper — same fix as #6.
 #8 [lint] AssertDiffSetsNeedsReplaceForForceNew — same fix as #6.
 #9 [lint] lintFile passed only the target file to the analyzers, so
    cross-file method sets were invisible (same blind spot the
    refactor-* modes had in round 1). Now lintFile loads sibling
    non-test .go files from the same package directory and feeds the
    full slice to each analyzer; diagnostics for sibling files are
    dropped (the outer walker visits them in their own turn) so no
    duplicate findings.

All 4 modes now compile-clean rewrites + honor 3-level skip-marker +
package-aware method-set detection.

* fix(codemod): Copilot review round 3 — 6 critical + 1 important findings

Round 3 surfaced 7 substantive findings; all addressed:

Critical (compile-break / silent data loss):

 #1 [add-validate-plan] Directory-wide detection only widened `provs`
    in round 2; methodsByRecv stayed file-local. A provider with
    ValidatePlan in a sibling file (or value-receiver Plan/Apply
    declared elsewhere) would receive a duplicate or wrong-receiver
    stub. Now planLikeProviderMethodsInDir returns both the recv set
    AND the merged method slice; methodsByRecv carries the
    package-wide view (deduped by method name). Stub injection still
    only fires when typeDecls[recv] is non-nil so we never append
    to a sibling file.
 #2 [refactor-plan] isCanonicalPlanBody accepted ANY 2-result return
    statement at the trailing slot. A planner with the canonical
    scaffold but a bespoke return (cloned plan, propagated error
    value) would classify as canonical and the bespoke logic would be
    silently dropped. Tightened to require EXACTLY `return plan, nil`.
 #3 [refactor-plan] rewritePlanBody hardcoded "desired"/"current" as
    args. A canonical Plan with renamed params (e.g. `Plan(ctx,
    specs, state)`) would rewrite to references to undefined
    identifiers. ensureNthParamName now extracts the actual signature
    names.
 #4 [refactor-plan] rewritePlanBody hardcoded "platform" as the call
    selector. A file using `pf "github.com/.../platform"` wouldn't
    compile because `platform` is undefined (ensureImport sees the
    aliased import as satisfying the path check). Added pkgAliasFor
    helper; rewrite now uses whatever local name the file imports
    under.
 #5 [refactor-apply] caseBodyIsCanonical accepted ANY AssignStmt as
    canonical. Bookkeeping AssignStmts (metrics counters, map
    updates, accumulators) passed and would be silently dropped.
    Tightened to a narrow whitelist: multi-target driver call,
    single-target driver call (LHS=err), composite-literal
    construction, selector-assignment to ResourceRef-style fields
    (ProviderID/Name/Type). Anything else rejected.
 #6 [refactor-apply] Same import-alias issue as #4 for `wfctlhelpers`.
    pkgAliasFor reused; rewriteApplyBody now uses whatever local name
    the file imports under.

Important:

 #7 [lint] AssertProviderImplementsValidatePlan checked ts.Doc only,
    missing markers placed on the wrapping GenDecl. Aligns now with
    the receiverDoc.carriesMarker pattern used by the other 3
    analyzers (round-2 #6/#7/#8). typeDocsByName captures both
    TypeSpec.Doc and GenDecl.Doc.

Round-2 regression tests retained (TestRefactorPlan_Fix_UnnamedReceiverGetsName,
TestRefactorPlan_Fix_PreservesCustomCtxName,
TestRefactorApply_Fix_UnnamedReceiverGetsName,
TestAddValidatePlan_Fix_ValueReceiverConvention).
Round-3 fix verified end-to-end against an aliased-import fixture
(pf "github.com/.../platform" + wfh "github.com/.../wfctlhelpers"):
the rewritten output compiles cleanly under gofmt.

* fix(codemod): Copilot review round 4 — 6 critical-detection-loosening findings

Round 4 surfaced 6 findings, all real. The recurring theme: rev3's
pattern detectors were either too loose (accepted bookkeeping shapes
as canonical) or too rigid (literal package-name matching, breaking
on aliased imports).

Fixes:

 #1 [add-validate-plan] interfacesQualifier(file) returned "" when the
    type-only file (no Plan/Apply imports) received the stub via
    cross-file detection (round-3 #1). Stub then emitted unqualified
    types that wouldn't compile. Now: when the file lacks an interfaces
    import but ANY sibling does, fall back to "interfaces" qualifier
    AND inject the interfaces import into the type-file via AST printing
    (format.Node) before appending the stub. Added
    siblingUsesInterfacesImport helper.
 #2 [refactor-apply] isCanonicalCaseAssign accepted ANY composite
    literal (`x := <CompositeLit>`) as canonical. A bookkeeping struct
    construction (audit payload, metric envelope) silently passed.
    Tightened to require the literal type's name (qualified or
    unqualified) match "ResourceRef".
 #3 [refactor-apply] isDriverMethodCall only checked selector NAME
    (Create/Read/Update/Delete). Calls like `helper.Update(...)` or
    `metrics.Delete(...)` were misclassified as canonical driver
    dispatch. Added receiver-allowlist check: only `d`, `drv`, or
    `driver` accepted as driver-bound identifiers (matching the
    standard `d, err := p.ResourceDriver(...)` pattern in DO/AWS/GCP/Azure).
 #4 [refactor-apply, refactor-plan] isAlreadyDelegatedApplyBody and
    isAlreadyDelegatedPlanBody required literal `wfctlhelpers` /
    `platform` package idents. Files using aliased imports
    (`wf "..."`, `pf "..."`) were misreported as non-canonical even
    though they were valid delegations. Both functions now resolve
    the file's local alias via pkgAliasFor; literal names retained
    as fallbacks. Same fix for isPlatformComputePlanAssign (the
    helper inside isAlreadyDelegatedPlanBody).
 #5 [lint] AssertPlanDelegatesToHelper / AssertApplyDelegatesToHelper
    selector matchers required literal `platform` / `wfctlhelpers`
    package names. Same false-positive risk as #4 for aliased imports.
    Both analyzers now resolve the alias and accept either the
    aliased OR literal form.
 #6 [refactor-apply] caseBodyIsCanonical accepted ANY DeclStmt as
    canonical, so `var x SomeBookkeepingType` declarations passed
    even though they're exactly the bespoke logic the codemod is
    supposed to preserve. Tightened via isLocalOutPointerDecl: only
    `var <name> *<ResourceOutput-suffix>` accepted.

Smoke-tested against an aliased-import fixture (`wf "...wfctlhelpers"`
+ `pf "...platform"`):
- refactor-apply correctly classifies as already-delegated (was:
  misreported as missing-action-switch)
- lint reports 0 findings (was: false-positive
  AssertPlanDelegatesToHelper + AssertApplyDelegatesToHelper)

* fix(codemod): Copilot review round 5 — 9 deeper-detection findings

Round 5 surfaced 9 findings; all addressed. Recurring theme: the
detectors and reporters needed deeper structural verification (branch
contents, outer-shape, receiver-kind, package isolation, exit-code
semantics) — not just shape matching at one level.

Critical (silent data loss / repair regression):

 #1 [refactor-plan] rangeBodyMatchesCanonicalDesired only checked the
    guard expressions and statement count; never inspected what the
    `!exists` and `configHash != configHash` branch BODIES did. A
    planner with extra logic (telemetry, alternate action construction,
    different create/update payload) inside those branches was
    silently rewritten away. Added isCanonicalCreateBranchBody +
    isCanonicalUpdateBranchBody + isPlanActionsAppendAssign to verify
    the create branch is exactly `append+continue` and the update
    branch is exactly `append`.
 #2 [refactor-apply] classifyApplyBody verified only the switch
    shape; setup/teardown/result aggregation OUTSIDE the switch was
    silently dropped on -fix. Added isCanonicalApplyOuterShape: the
    Apply body must be exactly the 3-statement scaffold (result-init
    + range-loop + return result, nil).
 #3 [add-validate-plan] hasValidatePlanMethod ignored receiver kind.
    A value-receiver provider with a pointer-receiver ValidatePlan
    still failed the ProviderValidator type assertion (method-set on
    `T` does not include `*T` methods), but rev2 treated it as
    already-implemented. Now also requires receiver-kind match.
 #4 [lint] AssertProviderImplementsValidatePlan had the same
    receiver-kind blind spot. Now delegates to hasValidatePlanMethod
    (centralised + DRY).
 #5 [refactor-plan] isAlreadyDelegatedPlanBody accepted single-statement
    `return platform.ComputePlan(...)` (broken rev1 form) as
    already-delegated, so rerunning the fixed codemod never repaired
    output from the earlier broken rewrite. Now ONLY accepts the
    canonical 2-statement form; broken single-statement forms classify
    as non-canonical so a fresh -fix produces compilable output.
 #6 [refactor-plan] planLikeProviderMethodsInDir merged methods from
    every non-test .go file regardless of `package P` clause. Mixed-
    package or build-tagged directories could fold methods from
    unrelated packages into a synthetic provider. Added two-pass
    package-clause check: aggregate only files matching the dominant
    package.

Important (CI fidelity / detector recall):

 #7 [Makefile, lint] `|| true` in migrate-providers swallowed real
    execution failures alongside expected advisory findings, because
    lint returned 1 for both findings AND parse errors. Split the
    exit codes: 0 clean / 1 findings / 2 errors. Makefile now gates
    on `[ $? -ne 0 ] && [ $? -ne 1 ]` so parse errors fail the
    target.
 #8 [refactor-plan] Canonical matcher hardcoded the lookup flag
    name as `exists`. The semantically-identical `cur, ok :=`
    idiomatic Go form was reported non-canonical. Widened to accept
    both `exists` and `ok`.
 #9 [refactor-apply] isDriverMethodCall allowlist {d, drv, driver}
    missed common alternates. Widened to {d, dr, drv, rdrv, driver,
    resourceDriver}. Still rejects bookkeeping receivers like
    `metrics`, `audit`, `helper` (preserves round-4 #3 fix).

End-to-end verification: lint against DO plugin produces exit 1 (3
advisory findings, no errors); broken-Go-source produces exit 2;
clean source produces exit 0. Smoke-tested via /tmp/iac-codemod.

* fix(codemod): Copilot review round 6 — type-doc skip-marker honored across sibling files

Round 6 surfaced 1 finding:

#1 [refactor-plan, refactor-apply, lint] receiverTypeDocs ran per-file
   only, so a `// wfctl:skip-iac-codemod` marker placed on a SIBLING
   file's type declaration was ignored when processing methods in the
   primary file. Round-3's directory-wide method-set scan made this
   layout possible (provider type in types.go, Plan/Apply in
   provider.go, skip-marker on the type), but the type-doc lookup
   wasn't widened in tandem. Effectively: providers explicitly opted
   out at the type-doc level were still rewritten if their methods
   were in a different file from the type.

Fix:

- Added receiverTypeDocsInDir(dir, primary) — merges receiverTypeDocs
  across every non-test .go file in dir whose `package P` matches the
  dominant package. Honors the same dominant-package filter introduced
  in round-5 #6 to keep build-tagged / mixed-package directories safe.
- refactor-plan + refactor-apply switched from receiverTypeDocs(file)
  to receiverTypeDocsInDir(filepath.Dir(path), file).
- lint's receiverTypeDocsForPass refactored to build a SINGLE merged
  map across pass.Files (which is already directory-wide after
  round-2 #9) and return it per-file. First-occurrence wins.

add_validate_plan unaffected: stub injection only fires when
typeDecls[recv] != nil (type IS in the current file), so its
skip-marker check on ts.Doc was never the cross-file scenario.

* fix(codemod): Copilot review round 7 — 6 cross-file + detection-tightening findings

Round 7 surfaced 10 findings; 4 were stale (already fixed in R6). 6
real findings addressed:

Critical (compile-break / silent data loss):

 #1 [refactor-plan] isPlanActionsAppendAssign verified the LHS but
    not append's first argument. A bespoke `plan.Actions =
    append(otherSlice, ...)` was misclassified as canonical and the
    alternate-slice logic silently dropped during rewrite. Now both
    LHS and append's first arg must reference plan.Actions.
 #3+#9 [refactor-apply] isCanonicalApplyOuterShape only checked the
    outer 3-statement scaffold; per-action logic INSIDE the for-loop
    body (logging, metrics, custom error handling, accumulators) was
    silently dropped on -fix. Added isCanonicalApplyLoopBody +
    isCanonicalApplyLoopAssign + isCanonicalApplyLoopIf +
    isCanonicalApplyLoopIfBodyStmt: every loop-body statement must
    match a tight whitelist (driver lookup, var-out decl, action
    switch, err-/out-guard ifs).
 #7+#8 [add-validate-plan] provs[recv].Pos() panicked when the
    TypeSpec was nil (cross-file scenario from round-3 #1: type
    declaration in sibling file). Now defaults Pos to NoPos for nil
    specs; sort still works (stable on name when Pos ties).

Important (cross-file consistency):

 #4 [add-validate-plan] qualifier fallback to "interfaces" fired
    based on whether ANY sibling imported interfaces — unreliable
    if THIS provider uses local types but an unrelated sibling
    imports interfaces. Replaced with qualifierFromProviderMethods:
    inspects the provider's OWN Plan/Apply parameter types
    (directory-wide via round-3 #1) for the qualifier they use.
 #5 [add-validate-plan] skip-marker check only consulted typeDecls
    (current file). When Plan/Apply are here but the type with
    `// wfctl:skip-iac-codemod` lives in a SIBLING file, the marker
    was ignored. Added siblingTypeDocs lookup via
    receiverTypeDocsInDir (the round-6 helper).
#10 [add-validate-plan] sibling-method merge deduped by method NAME
    only. If local file has wrong-signature ValidatePlan and sibling
    has correct one, sibling dropped, hasValidatePlanMethod saw only
    bad declaration, injected duplicate stub. Replaced with
    isLocalDuplicate: dedupes by name + parameter arity + result
    arity, so distinct signatures both survive.

Stale findings (already fixed in R6, no action needed):
 #2 refactor-apply receiverTypeDocsInDir already in place
 #6 lint receiver-doc lookup already merged via receiverTypeDocsForPass

Smoke-tested against DO plugin: refactor-plan reports DOProvider.Plan
canonical, refactor-apply reports DOProvider.Apply upsert-recovery
with the upsertSupporter suggestion. Output matches T8.7 baseline.

* fix(codemod): Copilot review round 8 — 9 dedup + skip-marker + identifier-name findings

Round 8 surfaced 9 findings; all addressed:

Critical (silent data loss / behavior change):

 #1 [add-validate-plan] isLocalDuplicate compared by name+arity only.
    Wrong-signature ValidatePlan(name string) []PlanDiagnostic and
    correct ValidatePlan(plan *IaCPlan) []PlanDiagnostic have same
    arity but different types — sibling-correct dropped, duplicate
    stub injected. Replaced with signature-fingerprint dedupe
    (signatureFingerprint + typeFingerprint walk all type shapes).
 #4 [refactor-apply] `default:` case clauses accepted without body
    inspection. Logging/metrics in default body silently dropped.
    Added isCanonicalDefaultBody: only `err = fmt.Errorf("unknown
    action %q", ...)` accepted.
 #5 [refactor-apply] isCanonicalApplyLoopAssign accepted any
    `<x>.ResourceDriver(...)`. `helper.ResourceDriver(...)` /
    `plan.ResourceDriver(...)` falsely classified. Now requires the
    receiver to match the provider's own receiver identifier
    (threaded through from classifyApplyBody).
 #8 [refactor-apply] Bare `if err != nil { continue }` accepted as
    canonical, but wfctlhelpers ALWAYS records ActionError before
    continuing — the rewrite would silently change behavior. Now
    requires the if-body to ALSO append to result.Errors before any
    continue/break.

Important (skip-marker scope + identifier flexibility):

 #2 [add-validate-plan] Skip-marker check fired on EVERY method's
    fn.Doc — a marker on Destroy/Status/etc. accidentally suppressed
    the whole provider's analysis. Restricted to Plan/Apply (the
    provider-defining methods).
 #3 [lint] AssertProviderImplementsValidatePlan — same fix as #2.
 #6 [refactor-plan] Canonical detector hardcoded `current`/`desired`
    body identifiers. Providers using `state`/`specs` reported
    non-canonical despite rewriter preserving names. Added
    nthParamName extraction; isCanonicalPlanBody now takes the actual
    parameter names.
 #7 [refactor-apply] Driver-receiver allowlist comment claimed `rd`
    accepted, but the switch was missing it. Added.
 #9 [refactor-apply] Canonical detector hardcoded `result` /`plan`
    identifier names. Providers using `res` /`pl` rejected. Now
    recovers actual identifier from signature (planName) and from
    statement-1 LHS (resultName); both must be consistent within the
    body but can be any identifier.

Smoke-tested against DO plugin: refactor-plan / refactor-apply still
report DOProvider.Plan canonical / DOProvider.Apply upsert-recovery
with stable upsertSupporter suggestion. Output matches T8.7 baseline.

Removed redundant TestRefactorApply_Fix_UnnamedReceiverGetsName: the
unnamed-receiver path can't have a canonical-shape Apply body
(`<recv>.ResourceDriver(...)` requires recv in scope). Receiver-name
injection is shared between refactor-plan and refactor-apply via
ensureReceiverName; coverage stays in
TestRefactorPlan_Fix_UnnamedReceiverGetsName.

* fix(codemod): Copilot review round 9 — 4 behavior-preservation findings

Round 9 surfaced 4 findings; all addressed:

Critical (silent behavior change):

 #1 [refactor-apply] If-guard body accepted bare `break`, but
    wfctlhelpers.ApplyPlan records the error and KEEPS processing
    later actions. A `break` would silently change loop semantics
    on rewrite. Now only `continue` is accepted in if-guard bodies.
 #2 [refactor-apply] Driver-method allowlist accepted `Driver` /
    `DriverFor` alongside `ResourceDriver`. wfctlhelpers dispatches
    SPECIFICALLY through IaCProvider.ResourceDriver; a wrapper like
    `provider.Driver(...)` would have its caching/instrumentation
    bypassed. Restricted to `ResourceDriver` only.

Important (false positives / cross-file alias mismatch):

 #3 [add-validate-plan, lint] Receiver-kind enforcement was too
    strict. Per Go spec, `*T`'s method set includes BOTH
    pointer-receiver and value-receiver methods of T. So a
    value-receiver ValidatePlan on a pointer-receiver provider IS
    valid (satisfies ProviderValidator). hasValidatePlanMethod now
    only requires strict matching when the provider uses VALUE
    receivers (T's method set excludes *T methods).
 #4 [add-validate-plan] When the qualifier was derived from a
    sibling method's aliased import (e.g. `iface "github.com/.../interfaces"`),
    the post-loop import injection used unaliased `ensureImport`,
    leaving the stub's `iface.IaCPlan` referring to undefined
    `iface`. Added ensureImportAs helper; now the import alias
    matches the stub's qualifier.

Smoke-tested against DO plugin: refactor-plan / refactor-apply still
report DOProvider.Plan canonical / DOProvider.Apply upsert-recovery
with stable upsertSupporter suggestion. Output matches T8.7 baseline.

* fix(codemod): Copilot review round 10 — 8 cross-file + perf + tightening

Round 10 surfaced 8 findings; all addressed:

Critical (cross-file duplicate stub / silent override):

 #1 [add-validate-plan] Cross-file duplicate stub injection: when type
    is in file_a and Plan/Apply are in file_b, both files classified
    as missing-ValidatePlan and -fix injected duplicate stubs. Now
    only inject in the file containing the receiver TypeSpec
    (`if ts == nil { skip }`); the type-file's own pass handles it.
 #2 [add-validate-plan] Embedded-field promoted ValidatePlan not
    detected; -fix would shadow it with a no-op stub, silently
    dropping real plan diagnostics. Added typeHasEmbeddedFields:
    if the receiver type has any embedded fields, suppress the
    missing classification (we can't statically resolve method
    promotion without full type info, so err on the side of NOT
    injecting).
 #3 [lint] AssertProviderImplementsValidatePlan — same fix as #2.
 #4 [refactor-apply] ProviderID/Name/Type assignment-target whitelist
    didn't check struct identity. `audit.Type = ...` or
    `result.ProviderID = ...` (wrong struct) classified as canonical
    and dropped on rewrite. Now requires the LHS receiver to be
    `ref` (the canonical ResourceRef construction site name).

Important (perf / determinism / lint precision):

 #5 [lint] O(n²) lintFile re-parsed every sibling per-call. Added
    lintDirCache: lintPath now groups files by directory and builds
    one parse cache per dir, reused across the directory's files.
    Per-call fallback retained for single-file invocation.
 #6 [refactor-plan] planLikeProviderMethodsInDir's dominant-package
    selection used range-over-map (random iteration), so on a
    package-count tie the dominant could differ across runs and
    rewrite against the wrong method set. Sort the package names
    so tie-break is lexicographic-first (deterministic).
 #7 [lint] AssertPlanDelegatesToHelper accepted ANY platform.ComputePlan
    call ANYWHERE in the body. Now requires the canonical SHAPE:
    either the 2-statement rev2 form (matches isAlreadyDelegatedPlanBody)
    OR a single-statement legacy `return <X>.Plan(...)` /
    `return <X>.ComputePlan(...)`. Bespoke wrappers that call the
    helper as an intermediate step now correctly flag.
 #8 [lint] AssertApplyDelegatesToHelper — same fix: now uses
    isAlreadyDelegatedApplyBody (the rewriter's idempotency check)
    so anything but the canonical single-statement form flags.

Smoke-tested against DO plugin: refactor-plan / refactor-apply still
report DOProvider.Plan canonical / DOProvider.Apply upsert-recovery
with stable upsertSupporter suggestion. Output matches T8.7 baseline.

* fix(codemod): Copilot review round 11 — 6 polish + revert findings

Round 11 surfaced 6 findings; all addressed:

Critical (broken-output false-clean / mode clobber):

 #1 [lint] planBodyDelegatesCanonically accepted single-statement
    `return platform.ComputePlan(...)` (the BROKEN rev1 form,
    uncompilable due to value/pointer mismatch). Lint reported
    partially-migrated providers as clean, so migrate-providers
    silently missed them. Now ONLY the canonical 2-statement rev2
    form OR legacy `return wfctlhelpers.Plan(...)` is accepted;
    the broken single-statement platform form falls through to
    non-canonical so lint surfaces the still-needs-fixup state.
 #2 [refactor-plan] writeFileAtomic left the temp file at
    os.CreateTemp's default 0600 mode; rename clobbered the
    source's original permissions (e.g., 0644 → 0600). Added
    writeFileAtomicBytesPreserveMode: captures original mode via
    os.Stat and chmods the temp file before rename.
 #5 [add-validate-plan] Same 0600 mode-clobber bug in
    writeFileAtomicBytes. Now delegates to
    writeFileAtomicBytesPreserveMode.

Important (revert + comment polish):

 #3 [add-validate-plan] Round-10 #2's "any embedded field suppresses
    missing-ValidatePlan" was too broad — sync.Mutex, loggers,
    config mixins don't promote ValidatePlan, so real targets were
    silently missed. Reverted: report missing unconditionally.
    Maintainers whose providers actually promote ValidatePlan
    suppress with the explicit `// wfctl:skip-iac-codemod` marker.
 #4 [lint] AssertProviderImplementsValidatePlan — same revert as #3.
 #6 [refactor-plan] Stale enum comment for planAlreadyDelegated still
    referenced `wfctlhelpers.Plan` as the recognised shape;
    actual implementation recognises the 2-statement
    platform.ComputePlan form. Comment updated.

Removed dead typeHasEmbeddedFields helper (both call sites reverted
in #3/#4). Source-file mode preservation verified end-to-end:
chmod 0644 → -fix → stat shows 0644 retained. Smoke-tested against
DO plugin: refactor-plan / refactor-apply still report
DOProvider.Plan canonical / DOProvider.Apply upsert-recovery with
stable upsertSupporter suggestion. Output matches T8.7 baseline.

* fix(codemod): Copilot review round 12 — 8 dispatcher + detection-tightening findings

Round 12 surfaced 8 findings; all addressed:

Critical (CLI bug + silent rewrite of wrong file):

 #1 [main.go] Top-level dispatcher used a single FlagSet with only
    -dry-run and -fix registered, so any mode-specific flag (e.g.
    refactor-apply's -report-file) failed with "flag provided but
    not defined" BEFORE the mode could parse it. -report-file was
    documented but UNUSABLE from the CLI entrypoint. Replaced
    stdlib FlagSet with a manual-scan loop in run(): -dry-run/-fix
    are extracted; everything else (including unknown flags) flows
    through to the mode's own FlagSet. Bonus: flag-position
    flexibility (`/path -fix` now works), updated test +
    usage text accordingly.
 #2 [refactor-plan] Walked every .go file but built provs/typeDocs
    from only the dominant package. Mixed-package or build-tagged
    directories: a non-dominant file with overlapping receiver
    names was processed against another package's method set,
    rewriting the wrong file. Added dominantPackageForDir; each
    file processor now skips files in non-dominant packages.
 #3 [refactor-apply] Same fix as #2.
 #4 [add-validate-plan] Same fix as #2.

Important (canonical-detection precision):

 #5 [refactor-plan] isPlanActionsAppendAssign didn't validate the
    appended action's payload — `plan.Actions = append(plan.Actions,
    PlanAction{Action: "queue"})` was misclassified as canonical and
    silently rewritten. Added `expectedAction` parameter; create
    branch requires `Action: "create"` and update branch requires
    `Action: "update"`.
 #6 [refactor-apply] hasCanonicalCases verified case labels but not
    that the body's driver call MATCHED the label. A `case "create"`
    body that called `.Update()` or `.Delete()` was misclassified
    and silently rewritten away. Added caseBodyMatchesLabel: scans
    each case body for driver method calls and verifies the label-
    to-method mapping (create→Create, update→Update, delete→Delete,
    replace→Update).
 #7 [refactor-apply] Driver-lookup check accepted any
    `<recv>.ResourceDriver(<arg>)` regardless of <arg>. wfctlhelpers
    always dispatches with `action.Resource.Type`, so providers
    using a different lookup key (e.g. action.Tag, computed value)
    would see different driver behavior on rewrite. Now requires
    the lookup key to be exactly `action.Resource.Type`.
 #8 [lint] looksLikeProvider checked method NAMES + rough arity,
    so any unrelated type with `Plan(...)` and `Apply(...)` was
    treated as a provider (e.g., a deploy strategy). Tightened to
    verify signature shapes via type-name suffix matching:
    Plan must be `Plan(ctx, []ResourceSpec, []ResourceState)
    (*IaCPlan, error)` and Apply must be `Apply(ctx, *IaCPlan)
    (*ApplyResult, error)`. Qualified or unqualified accepted via
    typeNameTailMatches.

Smoke-tested:
- `iac-codemod refactor-apply -report-file <path> <dir>` now works
  (previously: "flag provided but not defined")
- DO plugin still reports DOProvider.Plan canonical / Apply
  upsert-recovery with stable upsertSupporter suggestion (T8.7
  baseline preserved)

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

…der (v0.27.0) (#588)

* docs(design): engine-side sensitive-output routing through secrets.Provider

Captures the design for v0.27.0 engine layer that routes ResourceOutput
fields flagged Sensitive (or declared via SensitiveKeys()) through the
configured secrets.Provider before state persistence — keeping plugins
platform-agnostic per ADR 0015 / user mandate.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(design): rev1 of engine-sensitive-output-routing — adversarial findings

Addresses adversarial-design-review cycle 1 findings:
- Critical: drop non-existent audit-secrets recovery; add audit-state-secrets
- Critical: drop Restore API (incompatible with write-only GitHub provider);
  use in-memory hydrated map for same-process hand-off
- Critical: explicit resourceName parameter on Route (not from out.Name)
- Important: enumerate all 5 state-write call sites + per-site routing policy
- Important: SensitiveKeys() stays display-masking-only; routing uses
  per-call Sensitive map exclusively
- Important: Read/Adoption/Refresh paths sanitize-only (no Set), preventing
  cache pollution
- Important: compensating Delete on Set-success/Save-failure (no orphan
  cloud resources)
- Important: MaskSensitiveForDiff prevents drift false-positives

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(plan): engine-sensitive-output-routing implementation plan

5-task plan covering: iac/sensitive package, persistResourceWithSecretRouting
helper + Apply call-site rewires (sites 1+2), sanitize-only Read paths
(sites 3+4+5), audit-state-secrets command + drift masking, docs.

Single PR; ~1.6k lines net.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(plan): rev1 of engine-sensitive-output-routing — adversarial findings

Addresses adversarial-design-review (plan phase) cycle 1 findings:

Critical:
- Type-system mismatch: helper takes infraStateStore (wfctl-internal),
  not interfaces.IaCStateStore. Matches existing call-site discipline.
- Removed dead currentInfraConfigFile() reference; cfgFile threaded as
  explicit new parameter through applyWithProviderAndStore +
  applyPrecomputedPlanWithStore.
- Tasks 2 + 3 add Step N.7.bis runtime-launch-validation smoke (build
  artifact + exercise without/with secrets cfg, capture transcripts).

Important:
- Task 4 split into Task 4 (audit-state-secrets only) + Task 5 (drift
  masking enumeration with explicit no-op clause). Plan now has 6 tasks.
- Step 2.5.1 adds explicit existing-test-update sub-step for
  syncInfraOutputSecrets/resolveInfraOutput signature ripple.
- Step 2.4 #5 preserves validateOutputProviderID call (regression guard).
- Step 2.4 #6 adds pre-flight detection of sensitive-emitting drivers
  before persistence loop (no partial-apply mid-stream).
- Step 2.4 #7 isolates compensation context (fresh 30s timeout).
- Task 6 §6.4 validates rollback claim against v0.26.x consumer.
- Public API contract section locks exported names from Task 1.
- Scope Manifest excludes infra.go:1101 import path with reasoning.

Minor:
- Documents string-only sensitive value limitation as v0.27.0 constraint.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(plan): normalize Task headings to ### per writing-plans spec

Alignment-check identified ## Task → ### Task normalization required by
the writing-plans skill format. Six tasks now match alignment-check's
forward + reverse trace and the Scope Manifest counts.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore: lock scope for engine-sensitive-output-routing (alignment passed)

Manifest section sha256: f7a8e190ca8bc264252d2609ba856bdd979b3a5eaa3265d0d33f1dc85055c361
Locked at: 2026-05-09T08:11:07Z

PR Count: 1
Tasks: 6
Branch: feat/engine-sensitive-output-routing

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(iac/sensitive): Route/Revoke/IsPlaceholder/MaskSensitiveForDiff helpers

New iac/sensitive package for engine-side routing of ResourceOutput
sensitive fields through secrets.Provider. Free functions, no struct.

Exported API (locked at v0.27.0):
- Route(ctx, provider, resourceName, *out) (sanitized, hydrated, err)
- Revoke(ctx, provider, resourceName, mergedKeys) error
- IsPlaceholder(v any) bool
- MaskSensitiveForDiff(driverKeys, desired, current) (map, map)
- Placeholder(resourceName, outputKey) string
- PlaceholderPrefix string
- SecretKey(resourceName, outputKey) string

Placeholder format "secret_ref://<resource>_<key>" is distinct from the
existing user-supplied "secret://<key>" config-reference convention.

Routing trigger is exclusively out.Sensitive[k]==true; SensitiveKeys()
remains a display-masking-only signal per design rev1.

23 unit tests cover happy/error/edge cases per plan §7.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(wfctl): persistResourceWithSecretRouting + Apply call-site routing

Introduces persistResourceWithSecretRouting helper that funnels both
state-write call sites in applyWithProviderAndStore + in-process apply.
Routes per-call Sensitive ResourceOutput fields through secrets.Provider
into "secret_ref://" placeholders before SaveResource.

Threading:
- applyWithProviderAndStore + applyPrecomputedPlanWithStore each gain
  cfgFile string + hydratedOut map[string]string parameters.
- applyInfraModules now returns (map[string]string, error) where the
  map contains routed-secret values for in-process hand-off to
  syncInfraOutputSecrets.
- syncInfraOutputSecrets + resolveInfraOutput accept a hydrated map to
  rehydrate sensitive.PlaceholderPrefix entries from same-process apply
  (works on write-only GitHub Actions provider).

Failure modes:
- Pre-flight detection: if any driver emits Sensitive outputs but no
  secrets.Provider is configured, hard-fail BEFORE per-resource
  persistence loop (no partial-apply).
- On SaveResource failure post-Set, compensateAfterSaveFailure with a
  fresh 30s context calls driver.Delete + provider.Delete to prevent
  orphan cloud resources + routed secrets.

Test ripple: every existing call site updated to pass nil/empty for the
new parameters. New test file infra_apply_sensitive_routing_test.go
covers Route happy path, NoProvider hard-fail, SaveFailure compensation,
NoSensitive pass-through, idempotent re-apply, pre-flight detection,
read-mode placeholder preservation + new-key drop.

Rollback: revert this commit; helper + call sites revert to prior
literal Outputs: r.Outputs shape in one diff.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(wfctl): sanitize-only routing for adoption + refresh paths

Read paths (adoptExistingResources, runInfraRefreshOutputs) now route
state writes through persistResourceWithSecretRouting in persistModeRead.
The helper inherits placeholders from prior state (via ListResources)
and drops newly-declared sensitive keys — never calls provider.Set
from a Read path (prevents cache pollution per design §4.4).

Refresh path uses driver.SensitiveKeys() as the per-call Sensitive map
source (refresh has no per-call Sensitive declaration; SensitiveKeys
is the static driver-declared signal that approximates intent).

Site 4 (resourceStateFromLiveOutput) is a builder, not a save site;
no change needed there. Sites 3+5 wired through the helper.

Rollback: revert this commit; sites revert to direct store.SaveResource.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(wfctl): infra audit-state-secrets command

Adds wfctl infra audit-state-secrets command per design §4.7. Distinct
from existing audit-secrets (which audits the secrets.generate config
block for anti-patterns). Audit-state-secrets walks state.Outputs vs.
secrets.Provider to detect:

- placeholder-without-routed-value findings (state has secret_ref://
  placeholder but provider does not have the secret)
- legacy plaintext findings (state value matches DefaultSensitiveKeys
  heuristic AND is plaintext rather than a placeholder)
- mistaken secret://... config-reference findings (user-config syntax
  leaked into a persisted state field)
- orphan secret findings (provider has a name matching <res>_<key>
  pattern; no state resource named <res>)

--prune deletes confirmed orphan secrets idempotently (safe to rerun).

Exit codes: 0 = no findings; 1 = findings; 2 = audit error.

For write-only providers (GitHub Actions Get returns ErrUnsupported),
the command emits structured ADVISORY lines for each placeholder it
cannot verify, but does not exit non-zero on those alone.

Rollback: revert this commit; command is additive.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(wfctl): documented drift-masking no-op for v0.27.0

Per-the design's drift-masking enumeration task (Task 5 of the
engine-sensitive-output-routing plan): grep enumeration of
driver.Diff/d.Diff call sites in cmd/wfctl/ + iac/ shows zero
production sites that see state.Outputs flowing into Diff. Only
iac/conformance/scenario_grpc_roundtrip.go matches and that's a
conformance-test tool that synthesizes its own desired/current.

This commit ships the documented no-op outcome: a comment block at
the top of cmd/wfctl/infra_apply_refresh.go (the closest applicable
file) explains the enumeration outcome and how to wire
sensitive.MaskSensitiveForDiff if a future call site lands.
sensitive.MaskSensitiveForDiff stays exported as the future hook.

Rollback: revert this commit; comment is additive.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs: sensitive-output routing + audit-state-secrets command

- DOCUMENTATION.md: new "Sensitive output routing" section explaining
  Sensitive flag, secret_ref:// placeholder format, failure modes,
  recovery path, read-path semantics, drift masking, cold-start
  consumers, and v0.27.0 string-only limitation.
- docs/WFCTL.md: command-tree update + new "infra audit-state-secrets"
  command reference.
- CHANGELOG.md: comprehensive Unreleased entry covering Added (engine
  routing, iac/sensitive package, audit-state-secrets command),
  Changed (signature ripple for applyWithProviderAndStore +
  applyPrecomputedPlanWithStore + applyInfraModules +
  syncInfraOutputSecrets + resolveInfraOutput, sanitize-only routing
  for adoption + refresh), Migration (greenfield + legacy guidance),
  Rollback (v0.26.x runbook with validation-deferred note).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(wfctl): address Copilot review + lint G117 on PR #588

Lint fix:
- infra_rotate_and_prune.go:121: //nolint:gosec G117 false-positive — access_key
  is the public DO Spaces identifier (analogous to AWS access key ID); the
  credential secret_key is stored separately per ADR 0017 split-storage.

Copilot review fixes:
1. applyFromPrecomputedPlan now returns the same-process hydrated routed-secret
   map; --plan path threads it into syncInfraOutputSecrets so rehydration works
   on write-only providers (GitHub Actions secrets etc.). runHydrated
   populated for both --plan and live-diff branches.

2. (paired with #1) runInfraApply captures hydrated map from applyFromPrecomputedPlan.

3. adoptExistingResources precomputes priorByName once per pass; new
   persistResourceWithSecretRoutingCachedPrior helper avoids per-resource
   ListResources scans (was O(n²) on filesystem-backed stores).

4. audit-state-secrets --prune now refuses noopStateStore (no iac.state
   configured) — prevents catastrophic deletion when every secret would
   appear orphan.

5. audit-state-secrets surfaces non-ErrNotFound/non-ErrUnsupported errors
   from prov.Get as findings (provider error class) instead of swallowing.

6. Orphan-detection collects observed sensitive-key names from state
   placeholders + DefaultSensitiveKeys() and prefers the longest matching
   suffix; driver-declared non-default sensitive keys are no longer
   misclassified as orphans (and --prune won't delete them).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>